Repository navigation
Conversation
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — docs-only change, no blocking findings.
This PR edits a single file, docs/reporting-row-storage.md (+10/-9), clarifying shared row-storage migration guidance: the obligation_id FK is created by neither Python nor the fixed JS @adcp/reporting-storage releases; older JS migrations may have, remedied by JS migration 0011 or the quoted DROP CONSTRAINT. The reviewer confirmed both edited passages are internally consistent and the quoted constraint name matches the refusal output.
No code, wire-shape, public-API, or spec-drift exposure. This is not an adopter-facing behavior doc (not SKILL.md/AGENTS.md/llms.txt/README.md). No findings.
Decision table: no critical/high findings (rows 1,6 n/a); gated_paths false (row 2 n/a); high_risk false (rows 3,5 n/a); no medium findings (rows 4,8 n/a); no no-auto-approve team match (row 7 n/a). Falls through to row 9 → approve. Note: review_decision is REVIEW_REQUIRED, but with gated_paths false this does not block approval.
KonstantinMirin
left a comment
There was a problem hiding this comment.
Thanks, this reads well, and I checked the claims against both codebases: Python creates no foreign key, adcp-client#3651 removed it and added migration 0011, and the quoted DROP statement is valid. Three small things, none blocking:
- "Fixed releases of JavaScript
@adcp/reporting-storage" (docs/reporting-row-storage.md:60,:513) has no version, and none is published yet: the latest is 0.3.0-beta.5 from Nov 3, before #3651. Maybe "releases that include migration 0011", with the version once it ships. - Migration 0011 drops only the default-named
adcp_reporting_row_sets_obligation_id_fkey, and the doc doesn't say how to run it. One line,npx adcp-reporting-storage migrate, plus "for any other name, run the statementready_row_storagereturns", would close it. - The refusal operators actually see (
row_storage.py:669-672) still recommends applying the Python SQL before the other SDK's migration and doesn't mention 0011. That's a code change, so it's probably a follow-up.
Clarify how to share row storage with fixed and older JavaScript reporting-storage releases. Existing databases with the legacy foreign key can apply JavaScript migration 0011 or use the documented quoted DROP statement.
The readiness refusal already returns an executable, schema-qualified statement with the actual quoted constraint name; its PostgreSQL regression covers names containing quotes.
Tested: 28 row-storage PostgreSQL tests; 4 cross-SDK interoperability tests; JavaScript-first migration followed by Python writes and JavaScript reads.
Related: #1495, adcontextprotocol/adcp-client#3651 (JavaScript fix), #1879 (next release line).